Conversation
This was accidentally removed in 713444d.
Instead of generating load/store tests based on the input filename - which no longer works given the expected input file structure of `stdarch-gen-arm` - add a simple global context option that SVE specs can set.
The `SvUndef` expression is no longer necessary as a `core::intrinsics::scalable::sve_undef` intrinsic has been introduced to produce an undefined SVE vector, used by `svundef*` intrinsics. Other intrinsics that used `SvUndef` now use the `svundef*` intrinsics.
`simd_reinterpret` was expected to be used when it was added as `transmute_unchecked` requires `Sized`, but scalable vectors are now `Sized` so `transmute_unchecked` can be used and `simd_reinterpret` was not added in rust-lang/rust#143924.
Matching the current behaviour for arguments, `auto_llvm_sign_conversion` should only be required for `as_unsigned` conversions, not `into` conversions.
|
Looks like this'll need another rustc patch to skip the "scalable vectors aren't supported on this architecture" error for doc builds |
|
rust-lang/rust#154850 will fix this |
…, r=JonathanBrouwer ast_validation: scalable vectors okay for rustdoc Scalable vector types in `core_arch` are cfg'd for aarch64 and for rustdoc, which can successfully document these types given any `--target` (`core_arch` CI uses `i686-unknown-linux-gnu`) - this shouldn't trigger the "scalable vectors not supported on arch" error. This fixes the CI failure in rust-lang/stdarch#2071.
…, r=JonathanBrouwer ast_validation: scalable vectors okay for rustdoc Scalable vector types in `core_arch` are cfg'd for aarch64 and for rustdoc, which can successfully document these types given any `--target` (`core_arch` CI uses `i686-unknown-linux-gnu`) - this shouldn't trigger the "scalable vectors not supported on arch" error. This fixes the CI failure in rust-lang/stdarch#2071.
…, r=JonathanBrouwer ast_validation: scalable vectors okay for rustdoc Scalable vector types in `core_arch` are cfg'd for aarch64 and for rustdoc, which can successfully document these types given any `--target` (`core_arch` CI uses `i686-unknown-linux-gnu`) - this shouldn't trigger the "scalable vectors not supported on arch" error. This fixes the CI failure in rust-lang/stdarch#2071.
…, r=JonathanBrouwer ast_validation: scalable vectors okay for rustdoc Scalable vector types in `core_arch` are cfg'd for aarch64 and for rustdoc, which can successfully document these types given any `--target` (`core_arch` CI uses `i686-unknown-linux-gnu`) - this shouldn't trigger the "scalable vectors not supported on arch" error. This fixes the CI failure in rust-lang/stdarch#2071.
Rollup merge of #154850 - davidtwco:scalable-vectors-rustdoc, r=JonathanBrouwer ast_validation: scalable vectors okay for rustdoc Scalable vector types in `core_arch` are cfg'd for aarch64 and for rustdoc, which can successfully document these types given any `--target` (`core_arch` CI uses `i686-unknown-linux-gnu`) - this shouldn't trigger the "scalable vectors not supported on arch" error. This fixes the CI failure in rust-lang/stdarch#2071.
7203b17 to
d473c9a
Compare
|
rust-lang/rust#154950 will fix the current CI failure |
library: no `cfg(target_arch)` on scalable intrinsics These intrinsics don't technically need to be limited to a specific architecture, they'll probably only make sense to use on AArch64, but this just makes it harder to use them in stdarch where it is appropriate (such as on `arm64ec`): requiring a rustc patch to land and be on nightly before stdarch work can proceed. So let's just not `cfg` them at all, they're perma-unstable anyway. Fixes CI failure in rust-lang/stdarch#2071
library: no `cfg(target_arch)` on scalable intrinsics These intrinsics don't technically need to be limited to a specific architecture, they'll probably only make sense to use on AArch64, but this just makes it harder to use them in stdarch where it is appropriate (such as on `arm64ec`): requiring a rustc patch to land and be on nightly before stdarch work can proceed. So let's just not `cfg` them at all, they're perma-unstable anyway. Fixes CI failure in rust-lang/stdarch#2071
library: no `cfg(target_arch)` on scalable intrinsics These intrinsics don't technically need to be limited to a specific architecture, they'll probably only make sense to use on AArch64, but this just makes it harder to use them in stdarch where it is appropriate (such as on `arm64ec`): requiring a rustc patch to land and be on nightly before stdarch work can proceed. So let's just not `cfg` them at all, they're perma-unstable anyway. Fixes CI failure in rust-lang/stdarch#2071
Rollup merge of #154950 - davidtwco:scalable-no-cfg, r=RalfJung library: no `cfg(target_arch)` on scalable intrinsics These intrinsics don't technically need to be limited to a specific architecture, they'll probably only make sense to use on AArch64, but this just makes it harder to use them in stdarch where it is appropriate (such as on `arm64ec`): requiring a rustc patch to land and be on nightly before stdarch work can proceed. So let's just not `cfg` them at all, they're perma-unstable anyway. Fixes CI failure in rust-lang/stdarch#2071
library: no `cfg(target_arch)` on scalable intrinsics These intrinsics don't technically need to be limited to a specific architecture, they'll probably only make sense to use on AArch64, but this just makes it harder to use them in stdarch where it is appropriate (such as on `arm64ec`): requiring a rustc patch to land and be on nightly before stdarch work can proceed. So let's just not `cfg` them at all, they're perma-unstable anyway. Fixes CI failure in rust-lang/stdarch#2071
This is a convenience macro used by the generated SVE intrinsics. Co-authored-by: Jamie Cunliffe <Jamie.Cunliffe@arm.com> Co-authored-by: Luca Vizzarro <Luca.Vizzarro@arm.com> Co-authored-by: Adam Gemmell <adam.gemmell@arm.com> Co-authored-by: Jacob Bramley <jacob.bramley@arm.com>
| } | ||
| }) | ||
| }; | ||
| let return_type_conversion = self |
There was a problem hiding this comment.
It's probably the case that this signed/unsigned conversion isn't needed at all. Doesn't really cause any harm though
| ); | ||
| } | ||
|
|
||
| // Newer intrinsics don't have `rustc_legacy_const_generics` - assume they belong at |
There was a problem hiding this comment.
We could add this attribute for SVE intrinsics - see rust-lang/rust#149654. Some have pointed out that legacy generics is prone to bugs, but:
- It aids porting from C in a small way
- RfL really wants const args. I'm not sure if they necessarily want it for SIMD intrinsics though.
|
rust-lang/rust#154984 will fix this CI failure, eventually I'll just have issues on the stdarch side :) |
d473c9a to
6a6098b
Compare
Add the SVE types (without any of the generated intrinsics) and empty modules where the generated intrinsics will be. Enables the `adt_const_params` crate feature that the generated intrinsics will use. Co-authored-by: Jamie Cunliffe <Jamie.Cunliffe@arm.com> Co-authored-by: Luca Vizzarro <Luca.Vizzarro@arm.com> Co-authored-by: Adam Gemmell <adam.gemmell@arm.com> Co-authored-by: Jacob Bramley <jacob.bramley@arm.com>
`Into::into` can't be used here because the implementations can't have the required target feature, so `SveInto` needs to be introduced and written by the generator
`core::ptr::from_exposed_addr` was renamed to `core::ptr::with_exposed_provenance` and so this link needs updated.
Thousands of lines of SVE intrinsic definitions.. Co-authored-by: Jamie Cunliffe <Jamie.Cunliffe@arm.com> Co-authored-by: Luca Vizzarro <Luca.Vizzarro@arm.com> Co-authored-by: Adam Gemmell <adam.gemmell@arm.com> Co-authored-by: Jacob Bramley <jacob.bramley@arm.com>
Following from previous commit, this commit only contains generated code from the SVE intrinsic specifications Co-authored-by: Jamie Cunliffe <Jamie.Cunliffe@arm.com> Co-authored-by: Luca Vizzarro <Luca.Vizzarro@arm.com> Co-authored-by: Adam Gemmell <adam.gemmell@arm.com> Co-authored-by: Jacob Bramley <jacob.bramley@arm.com>
Co-authored-by: Adam Gemmell <Adam.Gemmell@arm.com> Co-authored-by: Jamie Cunliffe <Jamie.Cunliffe@arm.com> Co-authored-by: Jacob Bramley <jacob.bramley@arm.com> Co-authored-by: Luca Vizzarro <Luca.Vizzarro@arm.com>
Co-authored-by: Adam Gemmell <Adam.Gemmell@arm.com> Co-authored-by: Jamie Cunliffe <Jamie.Cunliffe@arm.com> Co-authored-by: Jacob Bramley <jacob.bramley@arm.com> Co-authored-by: Luca Vizzarro <Luca.Vizzarro@arm.com>
arm64ec doesn't support SVE.
With SVE intrinsics in the `arm_intrinsics.json`, the parsing needs to be updated to know to expect any new fields.
crates/core_arch/src/macros.rs
Outdated
|
|
||
| #[allow(unused_macros)] | ||
| macro_rules! static_assert_range { | ||
| ($imm:ident, $min:literal, $max:literal) => { |
There was a problem hiding this comment.
($imm:ident, $min:literal..=$max:literal) would make it clearer that this is an inclusive range.
|
|
||
| macro_rules! impl_sve_type { | ||
| ($(($v:vis, $elem_type:ty, $name:ident, $elt:literal))*) => ($( | ||
| #[derive(Clone, Copy)] |
There was a problem hiding this comment.
Have you checked whether this actually compiles when built as part of core? I expect this to fail because these public types have no documentation on them, which would trigger a denied lint.
There was a problem hiding this comment.
I copied over the lints that are enabled in core and checked that they pass - added some documentation comments to do that
crates/core_arch/src/lib.rs
Outdated
| @@ -1,4 +1,7 @@ | |||
| #![doc = include_str!("core_arch_docs.md")] | |||
| // FIXME(scalable-vectors): Required for `adt_const_params` Used for unit-only enums in SVE | |||
| // intrinsics. | |||
There was a problem hiding this comment.
These could also be defined as plain integer consts instead. In the end I don't mind either way but it may delay stabilization if we are blocked on adt_const_params.
There was a problem hiding this comment.
I've changed this to min_adt_const_params and removed incomplete_features - I've checked with @BoxyUwU and this should be pretty close to stabilisation so I feel comfortable relying on it for now
6a6098b to
9401448
Compare
I've closed this and changed the patch to only add these intrinsics on |
This is a very large patch but the vast majority of it is generated code, the rest should be relatively straightforward to review.
This passes tests for me locally with the latest nightly after rust-lang/rust#153286 has landed.
I've bolded the commits that do the most interesting things - the rest are largely updating the support that has been partially upstreamed in the past to work with the current implementation in rustc.
stdarch-verifyfor Arm - it was accidentally removed in 713444d.sve.spec.ymlandsve2.spec.ymland instead adds a key that can be set in those files to achieve the same behaviour.SvUndefexpressions from the generator, these were used insvundefintrinsics and some others.svundefnow zeroes and so doesn't need a special generator expression, and the other uses callsvundef.simd_reinterpretin the generator totransmute_unchecked-simd_reinterpretwas expected to be added byrustc_scalable_vector(N)rust#143924 but was changed during reviews.auto-llvm-sign-conversionin the generator for return types to match what it does for arguments.auto-llvm-sign-conversionisn't used by any existing intrinsic specification files.static_assert_rangehelper that will be used in later commits.SveInto::sve_into(a trait introduced in the previous commit). This trait is necessary becausetarget_featureannotations are necessary on the trait method and that requires the method be unsafe, which is incompatible with the signature ofInto::into.stdarch-verify. Load/store tests will also be generated from these definitions.intrinsics_data/arm_intrinsics.jsonfile to add all the SVE intrinsics so thatstdarch-verifycan check all of the intrinsics are present.stdarch-verifywith the necessary support and special-cases for SVE intrinsics.intrinsic-testtool so that it can still parsearm_intrinsics.jsonand skips the SVE intrinsics.The intrinsic test tool needs a lot of changes to work with SVE and this patch is getting big as it is, so we're going to do this as a follow-up (agreed in advance with @Amanieu).
This patch is based on #1509 and I've tried to preserve the co-authorship of everyone involved in that patch.
r? @Amanieu